-
Notifications
You must be signed in to change notification settings - Fork 6.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: move from angular-cli to gulp #948
Conversation
f2bc3ad
to
957b53e
Compare
Please, share the reasons behind moving to gulp build. Are you willing to stay with Systemjs or cli is very unstable now? |
@RemKolomna CLI is moving to WebPack, and as much as it pains me, it's not suitable for libraries (yet). We'll come back to using it when it does. |
6ede3f8
to
48627ea
Compare
@hansl glad to know they're going to use webpack Mind to share with us why you believe webpack isn't suitable for libraries yet? Just curious... |
I don't think this PR is the right place for this discussion so I'll be short; Angular-CLI is not suitable for libraries (yet). Webpack is suitable for apps, not libraries, because it's not meant to output files that will be published to npm. It's meant to bundle/pack files that will be served. I might move our demo-app serving and building to webpack though as it's faster than what we have here, but for now this fills my current set of goals to not be dependent on the CLI. |
5b3ba78
to
b6f11e4
Compare
225906e
to
c6858c8
Compare
@jelbourn: Ready for a second pass, and you'll love it; I added a |
'use strict'; | ||
const path = require('path'); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment here pointing people to the appropriate location for the tasks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -13,14 +13,13 @@ const config = { | |||
useAllAngular2AppRoots: true, | |||
specs: [ path.join(__dirname, '../e2e/**/*.e2e.ts') ], | |||
baseUrl: E2E_BASE_URL, | |||
allScriptsTimeout: 30000, | |||
getPageTimeout: 30000, | |||
allScriptsTimeout: 120000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a TODO to lower this back down?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll only have more tests, so I don't see a future where this will go down significantly.
LGTM aside from last few comments |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.